Skip to content

Conversation

@andrew-d
Copy link
Member

@andrew-d andrew-d commented Jan 7, 2026

If a hook is given, then a runtime.AddCleanup is registered for the []byte returned from ColumnBlob that verifies whether it's been modified. If so, the hook function is called with the corresponding query. Also, add tests to confirm that this works as expected.

Updates tailscale/corp#35671

If a hook is given, then a `runtime.AddCleanup` is registered for the
[]byte returned from ColumnBlob that verifies whether it's been
modified. If so, the hook function is called with the corresponding
query. Also, add tests to confirm that this works as expected.

Updates tailscale/corp#35671

Signed-off-by: Andrew Dunham <[email protected]>
@andrew-d andrew-d requested review from bradfitz and raggi January 7, 2026 20:47
@andrew-d
Copy link
Member Author

andrew-d commented Jan 7, 2026

Not sure we want to do this, but @raggi mentioned this in Slack and I figured it'd be interesting to try out 😃 I'm going to run this commit against the full corp testsuite and see what happens.

Copy link
Member

@raggi raggi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I mean I'm not sure if we have a strong reason to believe this is where we need to dig but if we did this a sampling way (some portion of queries) it might give some signal.

// TODO: We use uintptr instead of []byte to avoid keeping the slice alive
// (which would prevent the cleanup from running); is that right?
type blobCheckArg struct {
original []byte // copy of original data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or instead of this bytes.Clone, maybe store a https://pkg.go.dev/hash/fnv#New64a hash of the original data?

// can ensure they've run.
checkRun := make(chan struct{}, 10_000) // high enough to never block
blobCheckHook = func() {
checkRun <- struct{}{}
Copy link
Member

@bradfitz bradfitz Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

select {
case checkRun <- struct{}{}:
default:
   panic("checkRun unexpectedly full")
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants